Skip to content

Conversation

kudomcho
Copy link

@kudomcho kudomcho commented Oct 6, 2025

implemented pitch size on tensor allocation for better memory alignment.

Copy link

netlify bot commented Oct 6, 2025

Deploy Preview for pytorch-fbgemm-docs ready!

Name Link
🔨 Latest commit 6ce50d4
🔍 Latest deploy log https://app.netlify.com/projects/pytorch-fbgemm-docs/deploys/68e542ed1072590008b27a3e
😎 Deploy Preview https://deploy-preview-4977--pytorch-fbgemm-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@meta-cla meta-cla bot added the cla signed label Oct 6, 2025
Copy link
Contributor

meta-codesync bot commented Oct 6, 2025

@q10 has imported this pull request. If you are a Meta employee, you can view this in D83994225.

with torch.cuda.device(dst_device):
inputs = [torch.randn(10, 20) for _ in range(num_inputs)]
pitch = True
if pitch:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kudomcho This logic is strange, we set pitch and immediately check its value..

Copy link
Author

@kudomcho kudomcho Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@q10 This is to check the assertion allclose in case pitching is enabled. Currently it forces to True to test the all to one for the pitch condition. Any preference on passing the pitch condition on the test argument?

Copy link
Contributor

@q10 q10 Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@q10 This is to check the assertion allclose in case pitching is enabled. Currently it forces to True to test the all to one for the pitch condition. Any preference on passing the pitch condition on the test argument?

Yes, could you make this an argument to the test method, and use hypothesis @given(...) to pass the selection in?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added use_pitched on the given at test_all_to_one_device and moved the modified the merged_embeddings_benchmark.py to do pitch calculation by adding --use_pitched.

@kudomcho kudomcho force-pushed the khanin/merged_pool_emb_opt branch from bd261b7 to 3a36197 Compare October 7, 2025 16:17
…h calculation for better memory alignment on merged_embeddings_benchmark.py
q10 pushed a commit to q10/FBGEMM that referenced this pull request Oct 7, 2025
Summary:
X-link: facebookresearch/FBGEMM#1998

implemented pitch size on tensor allocation for better memory alignment.


Differential Revision: D83994225

Pulled By: q10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants